Repository navigation
test: compile thermochem kernels with the configured Fortran compiler - #1943
Conversation
test_thermochem.py hard-coded shutil.which("gfortran") and GNU flags. On
Frontier, ./mfc.sh load selects CCE (ftn) and leaves gfortran as the system
GCC 7.5, so ./mfc.sh lint failed test_precision_and_directives[dp-acc,dp-mp]:
GCC 7.5 rejects `declare target device_type(any)` and cannot link its nvptx
offload image. GitHub's ubuntu-latest has a modern gfortran, so CI never saw it.
Pick $FC, else ftn, else gfortran (as CMake does), identify the family from
--version, and use that family's flags and MFC_COMPILER define. GNU and Cray
are supported; other compilers skip with a message. CCE's OpenMP variant skips
without a craype-accel module, which declare target requires. The falloff
driver now writes with an explicit format, since Cray's list-directed output
is not parseable by np.fromstring.
The precheck hook now sets MFC_SKIP_COMPILER_TESTS=1, as it does
MFC_SKIP_RENDER_TESTS, so whether a commit passes no longer depends on the
modules loaded in the committer's shell. CI's ./mfc.sh lint still runs them.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 2
Open (3)
What changed in this PR
Updates thermochemistry kernel compilation tests to use the Fortran compiler/configuration selected by the toolchain (rather than hard-coded gfortran), improving portability on systems like Frontier/CCE and making local prechecks independent of loaded modules.
Changes:
- Detect the active Fortran compiler (
$FC, elseftn, elsegfortran), infer compiler family, and apply family-specific compile/offload flags and fypp defines. - Add skip controls for compiler-dependent tests (
MFC_SKIP_COMPILER_TESTS) and special-case skipping for Cray OpenMP offload when no accelerator module is loaded. - Make thermochem driver output deterministic/parseable across compilers by switching from list-directed
print *to an explicit format.
| File | Description |
|---|---|
| toolchain/mfc/test_thermochem.py | Uses configured Fortran compiler/family flags (GNU/Cray), adds skip logic, and stabilizes driver output formatting. |
| toolchain/bootstrap/precheck.sh | Skips Fortran-compilation tests during local precheck via MFC_SKIP_COMPILER_TESTS=1. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
toolchain/mfc/test_thermochem.py(added in #1915) hard-codedshutil.which("gfortran")and GNU flags. On Frontier,source ./mfc.sh load -c f -m gselects CCE (ftn) and leavesgfortranas the system GCC 7.5, so./mfc.sh lintfailedtest_precision_and_directives[dp-acc]and[dp-mp]:dp-mp: GCC 7.5 rejects!$omp declare target device_type(any)(fromomp_macros.fpp).dp-acc: GCC 7.5 compiles but fails to link its nvptx offload image.CI never saw this because the only jobs that run
./mfc.sh lintare on GitHub-hostedubuntu-latest, which ships a modern gfortran.Changes
$FC, elseftn, elsegfortran. Identify the family from--versionand use that family's flags plus theMFC_<id>/MFC_COMPILERfypp defines, ascmake/Fypp.cmakedoes.-ffree-line-length-none(ascmake/GPU.cmake).-eZ,-hacc/-hnoacc,-fopenmp,-Ktrap=divz,inv,ovf.craype-accelmodule is loaded (CRAY_ACCEL_TARGETunset); CCE rejectsdeclare targetwithout one.print *output isn't parseable bynp.fromstring.precheck.shsetsMFC_SKIP_COMPILER_TESTS=1, like the existingMFC_SKIP_RENDER_TESTS, so whether a commit passes no longer depends on the modules loaded in the committer's shell. CI's./mfc.sh lintstill runs these tests.Testing (Frontier login node)
source ./mfc.sh load -c f -m g && ./mfc.sh lint: 767 passed (was 2 failed on master)../mfc.sh lintwithout the load: 766 passed, 1 skipped (Cray OpenMP variant).FC=gfortran(system GCC 7.5) with the load: only the acc/mp variants fail, as expected for a compiler that can't build MFC's GPU directives.Acknowledgement